Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Декоративные элементы главной страницы #7

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mamifr
Copy link
Contributor

@mamifr mamifr commented May 15, 2022

@keksobot keksobot changed the title добавляет адаптивные элементы главной страницы Декоративные элементы главной страницы May 15, 2022
Copy link
Collaborator

@andreysgra andreysgra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Реализация состояния меню не соответствует описанию критерия Б30: <Без JS меню должно быть открыто по умолчанию и находиться в потоке (то есть не перекрывать остальной контент). Это изначальное состояние меню, а абсолютное позиционирование поверх всего контента реализуется в отдельном классе, который будет использоваться совместно с JS.

@@ -15,30 +15,30 @@
<div class="main-header-container">
<div class="header-header">
<div class="wrapper">
<nav class="main-nav">
<nav class="main-nav main-nav--opened">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В текущем состоянии вёрстки класс main-nav--opened никакой смысловой нагрузки не несёт: а) для него не определены стили; б) он должен применяться к тегу только в коде JS. Изначально в вёрстку его добавлять не нужно. В этом классе нужно задать свойства для абсолютного позиционирования меню поверх контента.

Другой класс main-nav--closed имеет противоположную функцию - в нём будет только одно свойство для скрытия меню. Класс также используется совместно с JS.

@@ -1,13 +1,15 @@
.header-header {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В своё время не комментировал этот момент: название класса крайне неудачное header-header. (Также нужно будет поменять название файла).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants